Skip to content

fix(terminal): continuation fragments of wrapped references are clickable - #175

Open
ghackett wants to merge 3 commits into
feat/wrapped-file-referencesfrom
fix/wrapped-tail-clicks
Open

fix(terminal): continuation fragments of wrapped references are clickable#175
ghackett wants to merge 3 commits into
feat/wrapped-file-referencesfrom
fix/wrapped-tail-clicks

Conversation

@ghackett

@ghackett ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member

Stacked on #174. A live testing pass there found that clicks on wrapped file references did nothing — and the failing half was the continuation fragment, the one holding the file name, i.e. exactly where you'd naturally click. Verified against a live headless terminal: with PR 174 as-is, only head-fragment clicks ever worked in Claude Code's real output format.

The two causes

  • Slashless continuations were never candidates. FILE_PATTERN deliberately requires a slash, and a wrap point usually falls inside the basename, so the continuation row shows o.py:7) — no VTE match under the pointer, and the click handler returned before the stitcher was consulted. Now, when nothing under the pointer matches, the raw whitespace-delimited token at the clicked cell stands in as the stitch candidate (token_at_column, unit-tested). The stitcher's geometry gates plus the existence check keep Ctrl+clicks on ordinary prose inert — verified live for row-edge prose tokens and empty areas.

  • Leading junk killed the join. The emitter wraps whatever surrounds the path along with it, so fragments arrive glued to prefixes — ⏺ Read(/a/b/c is Claude Code's own tool-call format, and the row-above token it contributes is Read(/a/b/fo, not /a/b/fo. The anchored match could only shed trailing punctuation, so every up-stitch through that format failed even when the continuation did contain a slash. Joins are now matched with a search over the joined string, with span guards requiring the hit to overlap the clicked fragment itself — one mechanism that sheds junk whether it sits in a contributed token or in a token-derived candidate, and that still refuses to resolve text the click never touched.

Verification

  • 5 new unit tests: up-stitch through a Read(-glued head, a slashless token candidate keeping its :7 suffix, a glued-prefix token candidate, the span guard (a junk-leading candidate must not resolve the row above's path), and token_at_column edge cases. 64 pass total.
  • Live headless run (real VTE, 40 rows of scrollback, pixel scrolling, Claude-style ⏺ Read(...)/⏺ Update(...) wrapped output): all 8 click scenarios pass — both fragments of a wrapped reference resolve with the line suffix, a head token glued to Update( resolves, plain unwrapped references still resolve, and prose tokens at row edges plus empty areas stay inert. Before the fix the same probe failed 4 of 8.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AXEJjBkzLLfVtVk6HhzFEn

ghackett and others added 2 commits August 3, 2026 10:00
…able

Live testing of the stitcher showed clicks on the second half of a
hard-wrapped path — the half holding the file name — did nothing, ever.
Two causes:

- A continuation fragment often contains no slash (`o.py:7)`), so
  FILE_PATTERN never matches it and the click handler bailed before the
  stitcher could run. When nothing under the pointer matches, the raw
  whitespace-delimited token at the clicked cell now stands in as the
  candidate; the stitcher's geometry gates and existence check keep prose
  clicks inert.

- Joins were matched with an anchored match, which can only shed trailing
  junk. The emitter wraps whatever surrounds the path along with it, so
  fragments arrive glued to prefixes — `Read(/a/b/c` is Claude Code's own
  tool-call format — and any leading junk killed the join. Matching now
  searches the join, with span guards so a hit must overlap the clicked
  fragment itself.

Verified live (headless, scrollback, pixel scrolling): both fragments of
`⏺ Read(collins/fo` + `o.py:7)` resolve with the line suffix, a head token
glued to `Update(` resolves, plain references still resolve, and prose
tokens at row edges stay inert. 5 new unit tests, 64 pass total.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AXEJjBkzLLfVtVk6HhzFEn
Live testing in the app showed every wrapped-reference click still dead.
Ground truth from running the real claude CLI inside a headless VTE: the
CLI's repaint-style renderer leaves VTE's internal ring a full page away
from what the adjustment describes (adjustment says rows 0-40, content
lives at ring rows 42-80), so every adjustment-derived get_text_range
read returned an empty row and the stitcher never saw any text. Feeding
text into a terminal keeps ring and adjustment aligned, which is why the
earlier probes never caught it.

Row texts now come from the visible-screen snapshot (get_text_format),
indexed by screen row straight from the click's y — no ring coordinates
involved. Soft-wrapped rows come back joined, so they are re-chunked at
the column count; pixel scrolling's partial top row is compensated by the
adjustment fraction. Trade-off: neighbour rows scrolled out of the
viewport are no longer reachable, which only matters for a wrapped
reference straddling the viewport edge.

Verified against the real CLI: a click map over every cell of a wrapped
absolute path (both the plain and the Read(...):42 form, echoed at 64
columns) resolves on all four rows, box borders and prose stay inert, and
a tilde path resolves to its directory. The feed-based probe (scrollback,
pixel scrolling) still passes all 8 scenarios; 24 unit tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AXEJjBkzLLfVtVk6HhzFEn
@ghackett

ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Follow-up from live testing: wrapped clicks were still dead in the app even with this branch. Ground truth from running the real claude CLI inside a headless VTE found the root cause — the CLI's repaint-style renderer leaves VTE's internal ring a full page away from what the adjustment describes (adjustment claimed rows 0–40 while the content sat at ring rows 42–80), so every adjustment-derived get_text_range read returned an empty row and the stitcher never saw any neighbour text. Feeding text into a terminal keeps ring and adjustment aligned, which is why the earlier probes never caught it.

Fixed in d5d86bd by reading row texts from the visible-screen snapshot (get_text_format) indexed by screen row straight from the click's y — no ring coordinates involved. A click map over every cell of a real Ink-rendered wrapped path (64 columns, both the bare and Read(...):42 forms) now resolves on all four rows, while box borders and prose stay inert.

A wrap-position sweep (every split of a long path, clicking every cell of
both rows) found one failing shape: the wrap landing right after a
character FILE_PATTERN refuses to end on — a head row ending `…/collins/`
or `…/collins/.`. The match candidate arrives without the shed run, so
the end-of-row gate never opened, stitching was skipped, and direct
resolution then opened the *parent directory* instead of the file — the
one hazard stitch-before-direct exists to prevent.

The gate now also opens when the row ends with the candidate plus a run
of exactly those shed characters, and the run seeds the downward join so
`…/collins` ⏎ `linkpatterns.py` stitches back to
`…/collins/linkpatterns.py`. Prose punctuation hanging off a reference at
a row end opens the gate too, but its joins resolve nowhere and the click
falls back to direct resolution as before.

3 new unit tests (27 pass); the 143-case sweep and the 8-scenario
feed probe pass clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AXEJjBkzLLfVtVk6HhzFEn
@ghackett

ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Second live-testing follow-up: continuation fragments containing a slash sometimes still misfired — clicking the head half opened the parent directory. A wrap-position sweep (all 143 viable splits of a long path, plain and Read(...):42 forms, clicking every cell of both rows) isolated the shape: the wrap falling right after a character FILE_PATTERN refuses to end on (…/collins/linkpatterns.py, or …/collins/.hidden/…). The candidate arrives without the shed /, the end-of-row gate never opens, and direct resolution then "succeeds" on the existing directory prefix — exactly the hazard stitch-before-direct was meant to prevent.

Fixed in 99d70f9: the gate also opens when the row ends with the candidate plus a run of exactly the shed characters (:.,;!?/), and that run seeds the downward join. Sweep now passes 143/143; 27 unit tests and the 8-scenario feed probe pass clean.

@ghackett
ghackett marked this pull request as ready for review August 3, 2026 16:56
@ghackett

ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @ghackett's task in 4m 44s —— View job


Review

  • Gather context (read diff, files, CLAUDE.md)
  • Review collins/linkpatterns.py changes
  • Review collins/terminal.py changes
  • Review new tests in tests/test_linkpatterns.py
  • Post review feedback

I read the full diff against origin/feat/wrapped-file-references and traced the logic by hand for every new test case (the two comment threads describe two rounds of real bugs already caught by live testing — good sign that this got exercised hard before review).

Correctness — no bugs found. I hand-traced resolve_wrapped_reference and token_at_column against all 8 new/changed test cases in tests/test_linkpatterns.py, including the trickier ones:

  • test_stitch_must_reach_into_the_candidate — confirmed the span guards (collins/linkpatterns.py:199-202) correctly reject the Read(/tmp/.../collins("x case: the only regex match found by finditer ends exactly at len(up), so m.end() <= len(up) correctly skips it, and the loop falls through to up="" / down="", which is skipped by the "bare candidate already failed" guard — net None, as asserted.
  • test_stitch_downward_across_directory_boundary / ..._hidden_dir_boundary — the _SHED_CHARS trailing-run logic (collins/linkpatterns.py:151-162) correctly recovers the shed / and /. and seeds the downward join.
  • test_prose_hanging_punctuation_does_not_false_join — confirmed the shed-chars gate can open on ordinary trailing punctuation (not just real wraps), but the join still fails existence resolution downstream ("collins/foo.py,and" doesn't exist), so the design is safe: the gate is permissive, the existence check is the real backstop.
  • test_stitch_upward_sheds_prefix_glued_to_head — confirmed the Read( prefix is shed for free: _PATH_PRE's lookbehind can't start a match inside Read(...)'s word-chars, so finditer's only viable match starts right after the ( boundary.

The terminal.py half — reading rows from get_text_format (visible-screen snapshot) instead of grid get_text_range_format, and folding the pixel-scroll fractional offset into y before the row = (y + frac) // ch division — matches what commits d5d86bd/99d70f9 describe fixing, and looks consistent with the rest of the click-handling code (the ±1 row slop, _STITCH_ROWS_UP/DOWN limits).

Minor, non-blocking nits:

  • collins/terminal.py:192 and :201_reference_roots(terminal) is now called twice per click in on_pressed (once in the new no-match branch, once in the kind == "file" branch). Could hoist above the if not uri: check to avoid the duplicate ancestor-widget lookup. Purely cosmetic, not worth blocking on.
  • collins/linkpatterns.py:150 / :171 — when trail == "" (candidate sits exactly at the row's end with no shed chars) and rows_below is empty or contributes an empty chain, downs ends up with a duplicate "" entry, so that combination gets tried twice. Harmless (same input, same result), just a trivial bit of redundant work.

Verification note: I wasn't able to execute the test suite directly in this environment (Bash tool calls require interactive approval that isn't available here), but get_ci_status shows the CI workflow for this PR already completed successfully (run 30829874417), consistent with the 64 unit tests and live headless-VTE probe described in the PR body and the two follow-up fix commits.

No changes requested — this looks solid and ready as-is.
· branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant